Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #36718 - Mark required fields for policy #545

Merged

Conversation

ofedoren
Copy link
Member

@ofedoren ofedoren commented Sep 4, 2023

No description provided.

@ofedoren ofedoren force-pushed the feat-36718-mark-policy-required branch from d02d8b9 to b8b2d0f Compare September 27, 2023 12:48
@ofedoren
Copy link
Member Author

Updated, I've overlooked the else branch for empty scap contents :/

Copy link
Contributor

@adamruzicka adamruzicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@ofedoren
Copy link
Member Author

ofedoren commented Sep 27, 2023

Test failures are not related, but look pretty scary :(

UPD: Weird, we have github actions, which run the same? set of tests and it passes, but jenkins does not...

@adamruzicka
Copy link
Contributor

Because there are github actions-based tests (which are green), I assumed the jenkins tests are a remnant of the past which we just failed to disable, but I could be wrong about that.

@ofedoren
Copy link
Member Author

Should we remove jenkins tests then? I'm not so familiar with this plugin and not sure we can rely on GA only...

@adamruzicka
Copy link
Contributor

Per #473 (comment) I would say yes

@ofedoren
Copy link
Member Author

ofedoren commented Oct 2, 2023

I've opened PR to remove jenkins job: theforeman/jenkins-jobs#360

@ekohl
Copy link
Member

ekohl commented Oct 2, 2023

Just for context, this is actually an issue we see in more plugins. There's a difference in how things run.

In GHA there is just a single "install" step. For Jenkins we first run Foreman, install & migrate (db:migrate & db:seed). Then the plugin is added and it's installed & migrated. Then Foreman also runs the entire test suite where I think GHA only runs the plugin specific tests.

The idea behind Jenkins is that it does regression testing and also testing the scenario where it's added to an existing Foreman. Whether it actually achieves those 2 things is a good question.

@stejskalleos and I also talked about this in the past, but I don't think we ever resolved it. I'd appreciate it if we looked at it from a broader project wide perspective.

@ofedoren
Copy link
Member Author

ofedoren commented Oct 2, 2023

Just for context, this is actually an issue we see in more plugins. There's a difference in how things run.

In GHA there is just a single "install" step. For Jenkins we first run Foreman, install & migrate (db:migrate & db:seed). Then the plugin is added and it's installed & migrated. Then Foreman also runs the entire test suite where I think GHA only runs the plugin specific tests.

The idea behind Jenkins is that it does regression testing and also testing the scenario where it's added to an existing Foreman. Whether it actually achieves those 2 things is a good question.

@stejskalleos and I also talked about this in the past, but I don't think we ever resolved it. I'd appreciate it if we looked at it from a broader project wide perspective.

I'll try to help, but no promises :/ Where should I start looking? I mean, it's obvious to look at the jenkin's console output for the run, but I haven't found anything useful, since I can't reproduce it locally unless (?) I'll create a new VM with the same steps jenkins has, but I guess nobody wants to spend time on that...

I mean, mostly I'd like to get rid of red status for jenkins here... And why it's not possible to do the same steps via GA?

@ekohl
Copy link
Member

ekohl commented Oct 2, 2023

I mean, mostly I'd like to get rid of red status for jenkins here... And why it's not possible to do the same steps via GA?

It is, but I'd like it if various plugins had some standard way of running CI. GHA is awful in the amount of duplication (by default) so I started https://github.com/theforeman/actions#foreman-plugin-tests. Never got around to completing it.

Then plugins that require Katello had a problem that they needed qpid headers, which weren't available on the Ubuntu based images. That's now gone, so it probably matters less.

AFAIK foreman_openscap doesn't require openscap itself, that's just smart_proxy_openscap. Otherwise you would also need to get libopenscap on Debian. Though I did see that Debian 12 has added libopenscap25.

Those are reasons why Jenkins still gives you better control.

As for the specific failures: I haven't tried to reproduce it myself yet. I think these would be roughly the steps:

git clone https://github.com/theforeman/foreman
cd foreman
# Make sure config/settings.yaml and config/database.yml are correct
bundle install
bundle exec rake db:migrate
bundle exec rake db:seed
cat > bundler.d/openscap.local.rb <<EOF
gem 'foreman_openscap', github: 'theforeman/foreman_openscap'
EOF
bundle install
bundle exec rake db:migrate
bundle exec rake db:seed

@ofedoren
Copy link
Member Author

ofedoren commented Oct 5, 2023

I've created an issue ticket to track Jenkins related issue: https://projects.theforeman.org/issues/36804, so we can go ahead with this PR, since it's more visual fix, not something that may introduce regressions.

@adamruzicka, could you please re-take a look? 👼

Copy link
Contributor

@adamruzicka adamruzicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this in

@adamruzicka adamruzicka merged commit 09d1197 into theforeman:master Oct 5, 2023
3 checks passed
@adamruzicka
Copy link
Contributor

Thank you @ofedoren !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants